Skip to content

Conversation

@adrianhaj
Copy link
Contributor

No description provided.

@adrianhaj
Copy link
Contributor Author

@jem-davies @gregfurman could I ask you for review this changes? I want to ask if it will be possible to release 1.11.1 with this changes ?

@jem-davies
Copy link
Collaborator

@jem-davies @gregfurman could I ask you for review this changes? I want to ask if it will be possible to release 1.11.1 with this changes ?

Thanks for the contribution 😎 - I would suggest considering implementing a plugin, while this PR is in review.

We don't typically add components in a patch release, and there are a few other PRs to review as well...

@gregfurman
Copy link
Collaborator

Hey! @adrianhaj 👋 Thanks for the super neat contribution 🤩 I'm busy reviewing this PR, and should be done by a bit later today.

Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fantastic PR! Really appreciate your usage of shutdown signallers to ensure this component cleans up gracefully 🤩

Genuinely a lot of fun to review, so kudos on the contribution 😄 Have left some comments and suggestions. Nothing really blocking but would like to hear your feedback on some of what I've raised.

a.locksMutex.Unlock()
}

func (a *azureServiceBusQueueReader) stopAllLockRenewals() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: I'm going to do a follow-up comment on this, but. think we can re-work this lock tracking using a singleflight.Group. Perhaps I'll open a seperate PR targeting this branch on your forked repo and we can see whether you think it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be grateful if you can 🙏

@adrianhaj
Copy link
Contributor Author

Thank you for the fantastic PR! Really appreciate your usage of shutdown signallers to ensure this component cleans up gracefully 🤩

Genuinely a lot of fun to review, so kudos on the contribution 😄 Have left some comments and suggestions. Nothing really blocking but would like to hear your feedback on some of what I've raised.

Thank you for review :) I will try to apply all changes as soon as possible :)

@adrianhaj
Copy link
Contributor Author

Thank you for the fantastic PR! Really appreciate your usage of shutdown signallers to ensure this component cleans up gracefully 🤩
Genuinely a lot of fun to review, so kudos on the contribution 😄 Have left some comments and suggestions. Nothing really blocking but would like to hear your feedback on some of what I've raised.

Thank you for review :) All changes applied. Please check it if any more comments I will be happy to contribute :)

@adrianhaj adrianhaj requested a review from gregfurman October 27, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants